Fix NavigationServer3D.get_closest_point_to_segment() with use_collision
#92850
Conversation
There was a problem hiding this comment.
Haven't tested but the change looks correct. I've checked with blame and it looks like the problem was there form the very beginning.
If you have some more time then I'd recommend adding a unit test covering that code path. There are unit tests for map already so it should be a matter of copy-pasting and modifying a bit.
permelin
force-pushed
the
fix-navigationserver3d-get_closest_point_to_segment-use_collision
branch
from
060a62c to
9b191f6
Compare
Well, if you had written tests instead of this TODO, the bug would have been found earlier. 😁
I've added a couple of tests. The subcase looks awkward, but I don't know what else to do. |
|
Good point. Maybe I was hesitant about that test case because it was giving me the wrong results :) |
akien-mga
changed the title
NavigationServer3D.get_closest_point_to_segment() with use_collision
|
Thanks! |
permelin
deleted the
fix-navigationserver3d-get_closest_point_to_segment-use_collision
branch
When calling
NavigationServer3D.get_closest_point_to_segment()with the parameteruse_collisionset to true, it always returns (0, 0, 0).The bug can be seen here, where
closest_point_dis incorrectly set to the same value asdon the first line, which hinders theelse ifbelow it from ever being true.godot/modules/navigation/nav_map.cpp
Lines 620 to 628 in e96ad5a
It's obvious that
closest_point_dandclosest_pointshould always be set together, so I can't imagine what the intention behind that first line could have been.(I find the way these lines are written to be confusing with unnecessary duplication, but I resisted the urge to refactor them.)